Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor NRL h2o photochemical physics scheme #213

Conversation

dustinswales
Copy link
Collaborator

Included in this PR are changes to NRL's stratospheric h2o photochemistry scheme to make it "safe across multiple CCPP instances". These changes are identical to the reorganization that o3 photochemistry scheme underwent in the fall of 2023..

Both schemes are called from a new module, GFS_photochemistry, at the start of the time-split section of the suite. Previously, the ozone scheme was called from GFS_stateout_update, and the h2o scheme was called just after.

Question @grantfirl @climbfuji
The o3 and h2o photochemistry schemes are both "time-splitting". Maybe this is a good place to start with the cleanup for MPAS effort (e.g. enforcing the procces-splitting requirement)?

Copy link
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job, looks good. Does this need to be high priority for some reason?

@dustinswales
Copy link
Collaborator Author

No rush on this.

@climbfuji
Copy link

Question @grantfirl @climbfuji The o3 and h2o photochemistry schemes are both "time-splitting". Maybe this is a good place to start with the cleanup for MPAS effort (e.g. enforcing the proccess-splitting requirement)?

I would not try to attempt commingling the changes from @michakales and framework-related development.

With this PR and the previous PRs for ccpp-framework and ccpp-physics, are all changes from the original PR included? Or is there still something missing?

@dustinswales
Copy link
Collaborator Author

Question @grantfirl @climbfuji The o3 and h2o photochemistry schemes are both "time-splitting". Maybe this is a good place to start with the cleanup for MPAS effort (e.g. enforcing the proccess-splitting requirement)?

I would not try to attempt commingling the changes from @michakales and framework-related development.

With this PR and the previous PRs for ccpp-framework and ccpp-physics, are all changes from the original PR included? Or is there still something missing?

@climbfuji The only piece missing from the OG PR is to move the Thompson mp "is_initialized" flag from module memory to GFS_typedefs. (This will avoid needing to bring in the "instance" field into the physics schemes.

@grantfirl
Copy link
Collaborator

@dustinswales Looks like there are some comments that need to be addressed before this is put in the queue?

@dustinswales
Copy link
Collaborator Author

@grantfirl Yeah. I've been meaning to circle back to this. I will update and address the comments later this afternoon.

@grantfirl
Copy link
Collaborator

@climbfuji Please re-review and see if 420317e addresses your concerns.

Copy link

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks very much!

@grantfirl
Copy link
Collaborator

@dustinswales I'm thinking of combining this with #218 to expedite testing/merging since this doesn't chance baselines. Is that OK with you?

[is_initialized]
standard_name = flag_for_thompson_mp_scheme_initialization
long_name = flag carrying scheme initialization status
units = flag
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here. Missing dimensions causing ccpp_prebuild to fail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I had un committed changes. My bad.

@grantfirl
Copy link
Collaborator

grantfirl commented Aug 23, 2024

@dustinswales Combined into #223 (Scheduled for UFS merge queue early next week)

@grantfirl grantfirl closed this Aug 23, 2024
dustinswales added a commit that referenced this pull request Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants